A lot of the changes are just mechanical changes of . to ->. I wrote a script to filter out these changes.

#!/usr/bin/env python3
import sys, re

AGGRESSIVE = any(a in ('-a', '--aggressive') for a in sys.argv[1:])

ANSI = re.compile(r'\x1b\[[0-9;?]*[a-zA-Z]')
def plain(s): return ANSI.sub('', s)                   # strip color for detection
def norm(l):  return plain(l)[1:].replace('->', '.')   # drop marker, normalize arrows

def hunk_is_noise(body):
    removed = [norm(l) for l in body if plain(l).startswith('-')]
    added   = [norm(l) for l in body if plain(l).startswith('+')]
    if not removed and not added:
        return False
    return sorted(removed) == sorted(added)            # every change is arrow-only

def aggressive_body(body):
    """Cancel only the arrow-noise -/+ pairs, keep everything else."""
    out, rem, add = [], [], []
    def flush():
        nonlocal rem, add
        used = [False] * len(add)
        anorm = [norm(a) for a in add]
        for r in rem:
            rn = norm(r); matched = False
            for i in range(len(add)):
                if not used[i] and anorm[i] == rn:      # same once arrows normalized
                    used[i] = matched = True
                    break
            if not matched:
                out.append(r)                           # a real removal, keep it
        out.extend(a for i, a in enumerate(add) if not used[i])
        rem, add = [], []
    for line in body:
        pl = plain(line)
        if pl.startswith('-'):
            if add: flush()                             # new change block began
            rem.append(line)
        elif pl.startswith('+'):
            add.append(line)
        else:
            flush(); out.append(line)                   # context / "\ No newline"
    flush()
    return out

out, file_header, file_header_emitted = [], [], False
hunk_header, hunk_body = None, []

def flush_hunk():
    global hunk_header, hunk_body, file_header_emitted
    if hunk_header is None:
        return
    if not hunk_is_noise(hunk_body):                    # drop fully-noise hunks entirely
        body = aggressive_body(hunk_body) if AGGRESSIVE else hunk_body
        if not file_header_emitted:
            out.extend(file_header); file_header_emitted = True
        out.append(hunk_header); out.extend(body)
    hunk_header, hunk_body = None, []

for line in sys.stdin:
    pl = plain(line)
    if pl.startswith('diff --git') or pl.startswith('diff --cc'):
        flush_hunk()
        file_header, file_header_emitted = [line], False
    elif pl.startswith('@@'):
        flush_hunk()
        hunk_header, hunk_body = line, []
    elif hunk_header is not None:
        hunk_body.append(line)
    else:
        file_header.append(line)

flush_hunk()
sys.stdout.write(''.join(out))

And then you can view the diff locally with: git diff $(git merge-base main HEAD) --color=always | python3 filter_arrows.py -a | less -R.

I would prefer if this was actually tool-driven (i.e. the clang-tidy check(s) enabled).

And if these are "non-null" pointers there is no reason to use pointers at al we should be getting a reference from the object. This makes things correct but worse as it looks like we have unchecked pointer dereferences all over the place again.

I have been working towards this for ages (with the focus on const correctness instead) in #4785 and cppcheck-opensource/simplecpp#548 but things stalled and I kept getting side tracked. Contributions on that would have been welcome.

I would prefer if this was actually tool-driven (i.e. the clang-tidy check(s) enabled).

Sorry, I somehow I overlooked this because I am feeling more under the weather than usual and should not be reviewing things.

And if these are "non-null" pointers there is no reason to use pointers at al we should be getting a reference from the object.

These only construct from the reference, so they cant be constructed as a null pointer. This is where it behaves like std::reference_wrapper and not gsl::non_null.

A reference cant be used as a member variable because they cant rebind(making the class non-copyable) where as NonNullPtr can rebind. The reason I named it as Ptr is because you need to dereference it like a pointer and it rebinds like a pointer.

I have been working towards this for ages (with the focus on const correctness instead) in #4785 and cppcheck-opensource/simplecpp#548 but things stalled and I kept getting side tracked.

I looked into making NonNullPtr propagate the const, but this would require a much larger change as some parameters need to remove const, and some of the loggers are being accessed non-const from const methods which requires a mutable variable(I dont know of an easy way to drop the mutable here).

However, there is one caveat with this. The const is still fairly shallow as you can just copy the variable and then modify it(this isnt a problem for ValuePtr because it copies a new value and not a reference). This is why propagate_const is non-copyable, but we cannot make NonNullPtr non-copyable as its whole purpose is to allow classes to be copyable.

I looked into making NonNullPtr propagate the const, but this would require a much larger change as some parameters need to remove const, and some of the loggers are being accessed non-const from const methods which requires a mutable variable(I dont know of an easy way to drop the mutable here).

I think this is mostly caused by ErrorLogger functions which should be const even if writing to the stream can be considered "technically not const".

Also we need to split the actual output from the ErrorLogger as we have code which just wants to output and has nothing to do with errors (like debug logging and dumping stuff). I have this prepared but as this is quite intrusive I haven't gotten around to it.

However, there is one caveat with this. The const is still fairly shallow as you can just copy the variable and then modify it(this isnt a problem for ValuePtr because it copies a new value and not a reference). This is why propagate_const is non-copyable, but we cannot make NonNullPtr non-copyable as its whole purpose is to allow classes to be copyable.

If you want to, you can get obvious get around it but it greatly improves things and helps the tooling to suggest more constness (see the simplecpp check).

These only construct from the reference, so they cant be constructed as a null pointer. This is where it behaves like std::reference_wrapper and not gsl::non_null.

A reference cant be used as a member variable because they cant rebind(making the class non-copyable) where as NonNullPtr can rebind. The reason I named it as Ptr is because you need to dereference it like a pointer and it rebinds like a pointer.

I meant we should not be providing a pointer from the class i.e. using . instead of ->. I do not like it at all that code is being to handling pointers instead of references - that always implies that it could be null although it is impossible.